Split tracing into vendor-neutral observability + OTel adapter#861
Conversation
Move all non-OTel tracing plumbing (Span/TracingProvider protocols, Noop implementation, SpanName, registry, YDB attribute helpers) into a new ydb.observability package. The OTel bridge in ydb.opentelemetry becomes a concrete provider (OtelTracingProvider) that plugs into the same interface. Users can now enable tracing without any OpenTelemetry dependency by supplying a custom TracingProvider to ydb.observability.enable_tracing. A second enable call always replaces the previously installed provider. ydb.opentelemetry.tracing is kept as a thin re-export shim for backward compatibility. All SDK internal imports (retries, connection, pool, session, transaction — sync + aio) now go through ydb.observability.tracing. Added tests/tracing/test_observability_enable.py covering: default Noop state, custom provider wiring, reset semantics on double enable, disable reverting to Noop, OTel enable replacing a custom provider, and duck-typed providers satisfying the protocol. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #861 +/- ##
==========================================
+ Coverage 81.38% 81.60% +0.22%
==========================================
Files 94 96 +2
Lines 12197 12228 +31
Branches 1204 1202 -2
==========================================
+ Hits 9926 9979 +53
+ Misses 1807 1799 -8
+ Partials 464 450 -14
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
- TestSplitEndpoint — parametric test covering every branch of _split_endpoint (grpcs://, grpc://, bare host:port, valid/malformed IPv6, missing colon, non-numeric port, None). - TestBuildYdbAttrsPeerOptionals — peer tuples with only address / only port / only location / empty location, hitting the three if x is not None branches in _build_ydb_attrs. - TestSetPeerAttributes — direct unit test for the set_peer_attributes helper (previously only reached through session flows that never had a non-None peer). - TestSpanFinishCallback — success and exception paths of the streaming _finish callback. - TestOpentelemetryTracingShimReexports — proves the ydb.opentelemetry.tracing back-compat shim re-exports the exact same objects as ydb.observability.tracing. - TestOtelEntrypointHandlesMissingPackage — uses sys.modules["ydb.opentelemetry.plugin"] = None to force the ImportError branches in ydb.opentelemetry.__init__.enable_tracing / disable_tracing. - TestOtelTracingSpanBridge — direct tests for TracingSpan.set_attribute and the defensive "exit before enter" branch in _AttachContext. - TestNoopContextManager — smoke tests for the Noop context manager and NoopTracingProvider returning the shared span + empty metadata.
There was a problem hiding this comment.
Pull request overview
This PR decouples SDK tracing from OpenTelemetry by introducing a vendor-neutral ydb.observability layer (interfaces + registry + helpers) and turning ydb.opentelemetry into a concrete adapter that installs an OtelTracingProvider via the new interface. This allows tracing to work without opentelemetry installed unless the user explicitly opts into the OTel adapter.
Changes:
- Add
ydb.observabilitywithSpan/TracingProviderprotocols, a Noop provider, a provider registry, and shared SDK tracing helpers (create_ydb_span, metadata injection, etc.). - Refactor SDK core (sync + async) to import tracing helpers from
ydb.observabilityinstead ofydb.opentelemetry. - Rework
ydb.opentelemetryinto an adapter (OtelTracingProvider) and keepydb.opentelemetry.tracingas a backward-compatible re-export shim; add new tests and update docs/changelog.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ydb/retries.py | Switch internal retry spans to ydb.observability.tracing. |
| ydb/query/transaction.py | Switch query transaction tracing helpers to ydb.observability.tracing. |
| ydb/query/session.py | Switch query session tracing helpers to ydb.observability.tracing. |
| ydb/pool.py | Switch pool tracing helpers to ydb.observability.tracing. |
| ydb/connection.py | Switch trace metadata injection source to ydb.observability.tracing. |
| ydb/aio/query/transaction.py | Maintain sync/async parity by switching async transaction tracing imports. |
| ydb/aio/query/session.py | Maintain sync/async parity by switching async session tracing imports. |
| ydb/aio/pool.py | Maintain sync/async parity by switching async pool tracing imports. |
| ydb/aio/connection.py | Maintain sync/async parity by switching async metadata injection imports. |
| ydb/observability/tracing.py | New vendor-neutral tracing protocols, Noop impl, registry, and shared SDK helpers. |
| ydb/observability/init.py | New public observability entrypoints (enable_tracing, disable_tracing, exports). |
| ydb/opentelemetry/plugin.py | Implement OtelTracingProvider adapter and route enable/disable through observability. |
| ydb/opentelemetry/init.py | Keep OTel entrypoints with lazy import and improved “replace provider” semantics. |
| ydb/opentelemetry/tracing.py | Preserve backward-compatible import paths via re-export shim to observability. |
| tests/tracing/test_tracing_sync.py | Update tracing tests to use observability registry and helpers. |
| tests/tracing/test_tracing_async.py | Update async tracing tests to import from observability. |
| tests/tracing/test_observability_enable.py | Add new unit tests for provider swapping/Noop behavior without OTel. |
| docs/opentelemetry.rst | Document provider replacement semantics and the new vendor-neutral API. |
| CHANGELOG.md | Note new ydb.observability entrypoint and removal of core OTel import dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
AI Review Summary
Verdict: ✅ No critical issues found
Critical issues
No critical issues found.
Other findings
- Minor | High: Incorrect import path in docstring —
ydb/observability/__init__.py:9 - Minor | Medium:
enable_tracingtype annotation rejectsNonebut implementation and docs accept it —ydb/observability/__init__.py:28 - Minor | Medium: Backward-compat shim re-exports
_registrybut its type changed fromOtelTracingRegistryto_TracingRegistrywith different methods —ydb/opentelemetry/tracing.py:17 - Nit | Low:
Span.attach_contextreturn type isAnyinstead ofContextManager—ydb/observability/tracing.py:39
This review was generated automatically. Critical issues require attention; other findings are advisory.
If this comment was useful, please give it a 👍 — it helps us improve the review bot.
|
Analysis performed by claude, claude-opus-4-6. |
…ability docs - Expose OtelTracingProvider from ydb.opentelemetry via lazy __getattr__ so the documented example imports without pulling opentelemetry at load time. - enable_tracing requires a provider (drop the None-as-disable overload); disable_tracing stays the single off switch. - Type fixes: headers as Dict[str, str], Span.attach_context -> ContextManager. - Docs: promote Observability to the umbrella page (interface, custom providers, the streaming end_on_exit contract); OpenTelemetry becomes a backend section.
…is on While a tracing provider is installed the SDK appends ydb-sdk-tracing/0.1.0 to the x-ydb-sdk-build-info header (native SDK, then SDK-owned feature tokens, then custom driver headers). The token list is aggregated by ydb.observability.sdk_build_info_tokens so future features (e.g. metrics) plug in with a single line.
| def __getattr__(name): | ||
| # Lazily expose the OTel provider so ``from ydb.opentelemetry import | ||
| # OtelTracingProvider`` works without importing ``opentelemetry`` at | ||
| # module load time. | ||
| if name == "OtelTracingProvider": | ||
| try: | ||
| from ydb.opentelemetry.plugin import OtelTracingProvider | ||
| except ImportError: | ||
| raise ImportError( | ||
| "OpenTelemetry packages are required for tracing support. " | ||
| "Install them with: pip install ydb[opentelemetry]" | ||
| ) from None | ||
| return OtelTracingProvider | ||
| raise AttributeError(f"module {__name__!r} has no attribute {name!r}") |
…tr__ Module __getattr__ raising ImportError broke hasattr()/getattr(default) introspection of OtelTracingProvider when OpenTelemetry is absent. Raise AttributeError instead, keeping the install hint in the message; enable_tracing() still surfaces its own ImportError guidance.
Summary
Fixes an architectural issue: tracing was tightly coupled to OpenTelemetry — the SDK forced users to have
opentelemetryinstalled just to enable tracing, and there was no way to plug in a custom backend.This PR splits the module in two:
ydb.observability(new) — vendor-neutral interfaces (Span,TracingProvider), Noop implementation,SpanName, registry, and all SDK-side helpers (create_ydb_span,set_peer_attributes,span_finish_callback,get_trace_metadata). The SDK core imports only from here and never touchesopentelemetry.ydb.opentelemetry— becomes a concreteOtelTracingProviderthat plugs into the same interface.ydb.opentelemetry.enable_tracing(tracer=None)is now a thin convenience that builds the provider and hands it toydb.observability.enable_tracing(...).New public API
enable_tracingcleanly resets the previously installed provider (works for OTel and custom providers alike — previouslyydb.opentelemetry.enable_tracingwas silently idempotent).ydb.opentelemetry.tracingremains as a re-export shim for backward compatibility.Tests
tests/tracing/test_observability_enable.py(12 tests) uses a hand-rolledRecordingProvider— no OTel involvement — to cover:enable_tracing,disable_tracingreverts to Noop,ydb.opentelemetry.enable_tracingreplacing a custom provider,TracingProviderprotocol.Test plan
tox -e black-format+tox -e blacktox -e styletox -e mypytox -e py -- ydb -v(174 passed)tox -e py -- tests/tracing -v(55 passed, including 12 new observability tests)🤖 Generated with Claude Code